Automated Test: guest-management-enhanced #368
Closed
+555
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* feat: ability to add guests via app.cal.com/bookings * fix: some update * fix: minor issue * fix: final update * update * update * add requested changes * fix type error * small update * final update * fix type error * fix location * update calender event --------- Co-authored-by: Somay Chauhan <somaychauhan98@gmail.com>
📝 WalkthroughWalkthroughThis change introduces functionality to add additional guests to existing bookings. It includes a new dialog UI component for entering guest emails, backend handler with permissions validation and calendar synchronization, email templates for notifying existing attendees and organizers, and a reusable MultiEmail input component. Changes
Sequence DiagramssequenceDiagram
participant User as User (Browser)
participant UI as BookingListItem
participant Dialog as AddGuestsDialog
participant tRPC as tRPC Mutation
participant DB as Database
participant Calendar as Calendar Service
participant Email as Email Service
User->>UI: Click "Additional Guests"
UI->>Dialog: Open dialog with bookingId
User->>Dialog: Enter guest emails
User->>Dialog: Click "Add"
Dialog->>Dialog: Validate emails (unique, valid)
Dialog->>tRPC: Call addGuests mutation
tRPC->>DB: Load booking + attendees + user
tRPC->>tRPC: Check permissions
tRPC->>tRPC: Filter new unique guests
tRPC->>DB: Add new attendees to booking
tRPC->>Calendar: Update calendar event attendees
tRPC->>Email: Send addGuests emails (organizer & attendees)
Email-->>User: Email notifications
tRPC-->>Dialog: Success response
Dialog->>Dialog: Show toast, close dialog
Dialog->>Dialog: Invalidate bookings cache
Dialog-->>User: Dialog closes, list refreshes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/web/components/dialog/AddGuestsDialog.tsx`:
- Around line 48-58: Trim and filter out empty strings from multiEmailValue
before validation and ignore validation if the resulting array is empty; use the
cleaned array when calling ZAddGuestsInputSchema.safeParse and when invoking
addGuestsMutation.mutate({ bookingId, guests }); also ensure isInvalidEmail is
cleared (call setIsInvalidEmail(false)) when the input changes and after a
successful mutation in handleAdd so the error state resets once the user fixes
entries.
- Around line 35-45: The error toast currently constructs message with a
template literal in addGuestsMutation.onError which causes "undefined:" to
appear when err.data?.code is missing; change the construction to only prefix
the code when present (e.g., if err.data?.code then `${err.data.code}:
${t(err.message)}` else use t(err.message) or fallback to
t("unable_to_add_guests")), then pass that safe string to showToast to avoid
emitting "undefined:".
- Around line 60-93: The dialog's onOpenChange currently points directly to
setIsOpenDialog so closing via overlay/escape doesn't clear multiEmailValue or
isInvalidEmail; change the handler for onOpenChange to a wrapper that calls
setIsOpenDialog(open) and when open is false resets state (call
setMultiEmailValue([""]) and setIsInvalidEmail(false)), or alternatively add a
useEffect watching isOpenDialog that clears multiEmailValue and isInvalidEmail
when isOpenDialog becomes false; update the Dialog prop (currently
onOpenChange={setIsOpenDialog}) to use this wrapper/behavior and ensure the same
reset logic used in the explicit Close button is reused.
In `@packages/emails/email-manager.ts`:
- Around line 525-550: sendAddGuestsEmails currently lacks the eventTypeMetadata
parameter and does not honor
eventTypeDisableHostEmail/eventTypeDisableAttendeeEmail like other email
functions (sendScheduledEmails, sendRescheduledEmails, sendCancelledEmails);
update sendAddGuestsEmails to accept an eventTypeMetadata argument and before
queuing emails, check eventTypeDisableHostEmail to skip organizer/teamMember
emails and eventTypeDisableAttendeeEmail to skip attendee emails (keeping
existing organizer/team member looping and attendee mapping logic but gated by
those flags), or alternatively add a clear comment in sendAddGuestsEmails
explaining why guest-add notifications intentionally bypass those disable flags.
In `@packages/emails/templates/organizer-add-guests-email.ts`:
- Around line 26-30: The subject construction in organizer-add-guests-email.ts
uses this.calEvent.attendees[0].name and can throw if attendees is empty; update
the subject building in the class/template to safely access the first attendee
(e.g., use a null-safe check or optional chaining like
this.calEvent.attendees?.[0]?.name) and provide a fallback string (e.g., "Guest"
or this.calEvent.organizer?.name) so subject creation (the template that calls
this.t("guests_added_event_type_subject", { eventType: this.calEvent.type, name:
..., date: this.getFormattedDate() })) never accesses undefined.
In `@packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts`:
- Around line 46-48: The permission boolean is currently computed as
isTeamAdminOrOwner using an AND of isTeamAdmin(...) && isTeamOwner(...), which
incorrectly requires both roles; change the logic to use OR so that
isTeamAdminOrOwner = await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)
|| await isTeamOwner(user.id, booking.eventType?.teamId ?? 0) so either role
grants permission (keep the same user.id and booking.eventType?.teamId ?? 0
operands).
- Around line 70-81: Normalize and dedupe incoming guest emails before computing
uniqueGuests: trim and lowercase each entry in guests (and derive a Set to
remove duplicates), compare against booking.attendees by normalizing
attendee.email to lowercase as well, and check against blacklistedGuestEmails
(which should also be normalized to lowercase/trimmed) so case-variant
duplicates or blacklisted variants are correctly rejected; update the logic
around uniqueGuests and the subsequent TRPCError throw to use these
normalized/deduped values.
- Around line 167-168: The code is sending emails using the original guests
array which may contain blacklisted, duplicate, or already-attending entries;
change the call to sendAddGuestsEmails to pass only the guests that were
actually validated/added (e.g., the result/variable returned by your add-guests
logic such as addedGuests or validatedGuests) by filtering out
blacklisted/duplicate/already-attending entries before invoking
sendAddGuestsEmails(evt, ...), or update sendAddGuestsEmails to accept and use
the validated/added list produced by the addGuests handler.
In `@packages/ui/form/MultiEmail.tsx`:
- Around line 20-25: The label with htmlFor="emails" is not linked to any input;
update the MultiEmail component to use a fieldset/legend for accessibility:
replace the standalone <label htmlFor="emails"> with a <fieldset> that contains
a <legend>{label}</legend>, wrap the mapped email inputs (the value.map
rendering) inside that fieldset, and ensure each rendered input remains
focusable and has any needed aria attributes (or ids) so screen readers
correctly associate the legend with the email fields.
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1)
3-6: Normalize and require at least one guest email in the schema.The schema currently accepts empty arrays and mixed-case emails. Normalizing and requiring at least one email helps reduce downstream dedupe/blacklist edge cases.
♻️ Proposed refactor
export const ZAddGuestsInputSchema = z.object({ bookingId: z.number(), - guests: z.array(z.string().email()), + guests: z + .array(z.string().trim().email().transform((email) => email.toLowerCase())) + .min(1), });apps/web/components/dialog/AddGuestsDialog.tsx (1)
26-29: Hoist the Zod schema to module scope.Right now the schema is recreated on every render; moving it out improves render efficiency and makes it reusable/testable.
♻️ Proposed refactor
+const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { + const uniqueEmails = new Set(emails); + return uniqueEmails.size === emails.length; +}); + export const AddGuestsDialog = (props: IAddGuestsDialog) => { const { t } = useLocale(); - const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { - const uniqueEmails = new Set(emails); - return uniqueEmails.size === emails.length; - });
Comment on lines
+35
to
+45
| const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({ | ||
| onSuccess: async () => { | ||
| showToast(t("guests_added"), "success"); | ||
| setIsOpenDialog(false); | ||
| setMultiEmailValue([""]); | ||
| utils.viewer.bookings.invalidate(); | ||
| }, | ||
| onError: (err) => { | ||
| const message = `${err.data?.code}: ${t(err.message)}`; | ||
| showToast(message || t("unable_to_add_guests"), "error"); | ||
| }, |
There was a problem hiding this comment.
Avoid emitting “undefined:” in the error toast.
message is always a non-empty string due to the template literal, so the fallback never runs and “undefined:” can leak into the UI.
🐛 Suggested fix
onError: (err) => {
- const message = `${err.data?.code}: ${t(err.message)}`;
- showToast(message || t("unable_to_add_guests"), "error");
+ const code = err.data?.code;
+ const text = err.message ? t(err.message) : t("unable_to_add_guests");
+ const message = code ? `${code}: ${text}` : text;
+ showToast(message, "error");
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({ | |
| onSuccess: async () => { | |
| showToast(t("guests_added"), "success"); | |
| setIsOpenDialog(false); | |
| setMultiEmailValue([""]); | |
| utils.viewer.bookings.invalidate(); | |
| }, | |
| onError: (err) => { | |
| const message = `${err.data?.code}: ${t(err.message)}`; | |
| showToast(message || t("unable_to_add_guests"), "error"); | |
| }, | |
| const addGuestsMutation = trpc.viewer.bookings.addGuests.useMutation({ | |
| onSuccess: async () => { | |
| showToast(t("guests_added"), "success"); | |
| setIsOpenDialog(false); | |
| setMultiEmailValue([""]); | |
| utils.viewer.bookings.invalidate(); | |
| }, | |
| onError: (err) => { | |
| const code = err.data?.code; | |
| const text = err.message ? t(err.message) : t("unable_to_add_guests"); | |
| const message = code ? `${code}: ${text}` : text; | |
| showToast(message, "error"); | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/components/dialog/AddGuestsDialog.tsx` around lines 35 - 45, The
error toast currently constructs message with a template literal in
addGuestsMutation.onError which causes "undefined:" to appear when
err.data?.code is missing; change the construction to only prefix the code when
present (e.g., if err.data?.code then `${err.data.code}: ${t(err.message)}` else
use t(err.message) or fallback to t("unable_to_add_guests")), then pass that
safe string to showToast to avoid emitting "undefined:".
Comment on lines
+48
to
+58
| const handleAdd = () => { | ||
| if (multiEmailValue.length === 0) { | ||
| return; | ||
| } | ||
| const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue); | ||
| if (validationResult.success) { | ||
| addGuestsMutation.mutate({ bookingId, guests: multiEmailValue }); | ||
| } else { | ||
| setIsInvalidEmail(true); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Clear invalid state on edits and ignore empty entries before validation.
Once isInvalidEmail flips to true, it never clears when the user fixes inputs. Also the default [""] will always fail validation. Normalize and filter empties, and reset the error on success (or on input change).
🛠️ Suggested adjustment
-const handleAdd = () => {
- if (multiEmailValue.length === 0) {
+const handleAdd = () => {
+ const emails = multiEmailValue.map((e) => e.trim()).filter(Boolean);
+ if (emails.length === 0) {
return;
}
- const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+ const validationResult = ZAddGuestsInputSchema.safeParse(emails);
if (validationResult.success) {
- addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
+ setIsInvalidEmail(false);
+ addGuestsMutation.mutate({ bookingId, guests: emails });
} else {
setIsInvalidEmail(true);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleAdd = () => { | |
| if (multiEmailValue.length === 0) { | |
| return; | |
| } | |
| const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue); | |
| if (validationResult.success) { | |
| addGuestsMutation.mutate({ bookingId, guests: multiEmailValue }); | |
| } else { | |
| setIsInvalidEmail(true); | |
| } | |
| }; | |
| const handleAdd = () => { | |
| const emails = multiEmailValue.map((e) => e.trim()).filter(Boolean); | |
| if (emails.length === 0) { | |
| return; | |
| } | |
| const validationResult = ZAddGuestsInputSchema.safeParse(emails); | |
| if (validationResult.success) { | |
| setIsInvalidEmail(false); | |
| addGuestsMutation.mutate({ bookingId, guests: emails }); | |
| } else { | |
| setIsInvalidEmail(true); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@apps/web/components/dialog/AddGuestsDialog.tsx` around lines 48 - 58, Trim
and filter out empty strings from multiEmailValue before validation and ignore
validation if the resulting array is empty; use the cleaned array when calling
ZAddGuestsInputSchema.safeParse and when invoking addGuestsMutation.mutate({
bookingId, guests }); also ensure isInvalidEmail is cleared (call
setIsInvalidEmail(false)) when the input changes and after a successful mutation
in handleAdd so the error state resets once the user fixes entries.
Comment on lines
+60
to
+93
| return ( | ||
| <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}> | ||
| <DialogContent enableOverflow> | ||
| <div className="flex flex-row space-x-3"> | ||
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | ||
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | ||
| </div> | ||
| <div className="w-full pt-1"> | ||
| <DialogHeader title={t("additional_guests")} /> | ||
| <MultiEmail | ||
| label={t("add_emails")} | ||
| value={multiEmailValue} | ||
| readOnly={false} | ||
| setValue={setMultiEmailValue} | ||
| /> | ||
|
|
||
| {isInvalidEmail && ( | ||
| <div className="my-4 flex text-sm text-red-700"> | ||
| <div className="flex-shrink-0"> | ||
| <Icon name="triangle-alert" className="h-5 w-5" /> | ||
| </div> | ||
| <div className="ml-3"> | ||
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <DialogFooter> | ||
| <Button | ||
| onClick={() => { | ||
| setMultiEmailValue([""]); | ||
| setIsInvalidEmail(false); | ||
| setIsOpenDialog(false); | ||
| }} |
There was a problem hiding this comment.
Reset dialog state when closing via overlay/escape.
Closing the dialog through onOpenChange doesn’t clear emails or the error state, so reopening can show stale inputs/error banners.
🧹 Suggested fix
-<Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}>
+<Dialog
+ open={isOpenDialog}
+ onOpenChange={(open) => {
+ if (!open) {
+ setMultiEmailValue([""]);
+ setIsInvalidEmail(false);
+ }
+ setIsOpenDialog(open);
+ }}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ( | |
| <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}> | |
| <DialogContent enableOverflow> | |
| <div className="flex flex-row space-x-3"> | |
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | |
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | |
| </div> | |
| <div className="w-full pt-1"> | |
| <DialogHeader title={t("additional_guests")} /> | |
| <MultiEmail | |
| label={t("add_emails")} | |
| value={multiEmailValue} | |
| readOnly={false} | |
| setValue={setMultiEmailValue} | |
| /> | |
| {isInvalidEmail && ( | |
| <div className="my-4 flex text-sm text-red-700"> | |
| <div className="flex-shrink-0"> | |
| <Icon name="triangle-alert" className="h-5 w-5" /> | |
| </div> | |
| <div className="ml-3"> | |
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | |
| </div> | |
| </div> | |
| )} | |
| <DialogFooter> | |
| <Button | |
| onClick={() => { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| setIsOpenDialog(false); | |
| }} | |
| return ( | |
| <Dialog | |
| open={isOpenDialog} | |
| onOpenChange={(open) => { | |
| if (!open) { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| } | |
| setIsOpenDialog(open); | |
| }}> | |
| <DialogContent enableOverflow> | |
| <div className="flex flex-row space-x-3"> | |
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | |
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | |
| </div> | |
| <div className="w-full pt-1"> | |
| <DialogHeader title={t("additional_guests")} /> | |
| <MultiEmail | |
| label={t("add_emails")} | |
| value={multiEmailValue} | |
| readOnly={false} | |
| setValue={setMultiEmailValue} | |
| /> | |
| {isInvalidEmail && ( | |
| <div className="my-4 flex text-sm text-red-700"> | |
| <div className="flex-shrink-0"> | |
| <Icon name="triangle-alert" className="h-5 w-5" /> | |
| </div> | |
| <div className="ml-3"> | |
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | |
| </div> | |
| </div> | |
| )} | |
| <DialogFooter> | |
| <Button | |
| onClick={() => { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| setIsOpenDialog(false); | |
| }} |
🤖 Prompt for AI Agents
In `@apps/web/components/dialog/AddGuestsDialog.tsx` around lines 60 - 93, The
dialog's onOpenChange currently points directly to setIsOpenDialog so closing
via overlay/escape doesn't clear multiEmailValue or isInvalidEmail; change the
handler for onOpenChange to a wrapper that calls setIsOpenDialog(open) and when
open is false resets state (call setMultiEmailValue([""]) and
setIsInvalidEmail(false)), or alternatively add a useEffect watching
isOpenDialog that clears multiEmailValue and isInvalidEmail when isOpenDialog
becomes false; update the Dialog prop (currently onOpenChange={setIsOpenDialog})
to use this wrapper/behavior and ensure the same reset logic used in the
explicit Close button is reused.
Comment on lines
+525
to
+550
| export const sendAddGuestsEmails = async (calEvent: CalendarEvent, newGuests: string[]) => { | ||
| const calendarEvent = formatCalEvent(calEvent); | ||
|
|
||
| const emailsToSend: Promise<unknown>[] = []; | ||
| emailsToSend.push(sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent }))); | ||
|
|
||
| if (calendarEvent.team?.members) { | ||
| for (const teamMember of calendarEvent.team.members) { | ||
| emailsToSend.push( | ||
| sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent, teamMember })) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| emailsToSend.push( | ||
| ...calendarEvent.attendees.map((attendee) => { | ||
| if (newGuests.includes(attendee.email)) { | ||
| return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee)); | ||
| } else { | ||
| return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee)); | ||
| } | ||
| }) | ||
| ); | ||
|
|
||
| await Promise.all(emailsToSend); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "eventTypeDisableHostEmail|eventTypeDisableAttendeeEmail" packages/emails/email-manager.ts -A 2 -B 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 5574
🏁 Script executed:
# Check the function signatures of the compared functions
rg -n "export const send(Scheduled|Rescheduled)Emails" packages/emails/email-manager.ts -A 3Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 381
🏁 Script executed:
# Look for all functions that use eventTypeMetadata parameter
rg -n "eventTypeMetadata" packages/emails/email-manager.ts | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1272
Add eventTypeMetadata parameter and email disable checks to maintain consistency.
All other email functions in this file (e.g., sendScheduledEmails, sendRescheduledEmails, sendCancelledEmails) accept eventTypeMetadata and check eventTypeDisableHostEmail and eventTypeDisableAttendeeEmail before sending emails. The sendAddGuestsEmails function should either:
- Accept
eventTypeMetadataand perform these checks for consistency with the rest of the codebase, or - Add a comment explaining why guest addition notifications bypass these disable settings.
🤖 Prompt for AI Agents
In `@packages/emails/email-manager.ts` around lines 525 - 550, sendAddGuestsEmails
currently lacks the eventTypeMetadata parameter and does not honor
eventTypeDisableHostEmail/eventTypeDisableAttendeeEmail like other email
functions (sendScheduledEmails, sendRescheduledEmails, sendCancelledEmails);
update sendAddGuestsEmails to accept an eventTypeMetadata argument and before
queuing emails, check eventTypeDisableHostEmail to skip organizer/teamMember
emails and eventTypeDisableAttendeeEmail to skip attendee emails (keeping
existing organizer/team member looping and attendee mapping logic but gated by
those flags), or alternatively add a clear comment in sendAddGuestsEmails
explaining why guest-add notifications intentionally bypass those disable flags.
Comment on lines
+26
to
+30
| subject: `${this.t("guests_added_event_type_subject", { | ||
| eventType: this.calEvent.type, | ||
| name: this.calEvent.attendees[0].name, | ||
| date: this.getFormattedDate(), | ||
| })}`, |
There was a problem hiding this comment.
Potential null pointer exception when accessing attendees[0].name.
If this.calEvent.attendees is empty, accessing this.calEvent.attendees[0].name on Line 28 will throw a runtime error. This could occur in edge cases where the calendar event has no attendees.
🛡️ Proposed fix: Add null safety check
subject: `${this.t("guests_added_event_type_subject", {
eventType: this.calEvent.type,
- name: this.calEvent.attendees[0].name,
+ name: this.calEvent.attendees[0]?.name ?? this.calEvent.organizer.name,
date: this.getFormattedDate(),
})}`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| subject: `${this.t("guests_added_event_type_subject", { | |
| eventType: this.calEvent.type, | |
| name: this.calEvent.attendees[0].name, | |
| date: this.getFormattedDate(), | |
| })}`, | |
| subject: `${this.t("guests_added_event_type_subject", { | |
| eventType: this.calEvent.type, | |
| name: this.calEvent.attendees[0]?.name ?? this.calEvent.organizer.name, | |
| date: this.getFormattedDate(), | |
| })}`, |
🤖 Prompt for AI Agents
In `@packages/emails/templates/organizer-add-guests-email.ts` around lines 26 -
30, The subject construction in organizer-add-guests-email.ts uses
this.calEvent.attendees[0].name and can throw if attendees is empty; update the
subject building in the class/template to safely access the first attendee
(e.g., use a null-safe check or optional chaining like
this.calEvent.attendees?.[0]?.name) and provide a fallback string (e.g., "Guest"
or this.calEvent.organizer?.name) so subject creation (the template that calls
this.t("guests_added_event_type_subject", { eventType: this.calEvent.type, name:
..., date: this.getFormattedDate() })) never accesses undefined.
Comment on lines
+46
to
+48
| const isTeamAdminOrOwner = | ||
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && | ||
| (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0)); |
There was a problem hiding this comment.
Permission check should be OR, not AND.
Current logic requires a user to be both team admin and owner, which blocks legitimate admins/owners.
✅ Suggested fix
- const isTeamAdminOrOwner =
- (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) &&
- (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
+ const isTeamAdminOrOwner =
+ (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) ||
+ (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isTeamAdminOrOwner = | |
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && | |
| (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0)); | |
| const isTeamAdminOrOwner = | |
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) || | |
| (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0)); |
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts` around
lines 46 - 48, The permission boolean is currently computed as
isTeamAdminOrOwner using an AND of isTeamAdmin(...) && isTeamOwner(...), which
incorrectly requires both roles; change the logic to use OR so that
isTeamAdminOrOwner = await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)
|| await isTeamOwner(user.id, booking.eventType?.teamId ?? 0) so either role
grants permission (keep the same user.id and booking.eventType?.teamId ?? 0
operands).
Comment on lines
+70
to
+81
| const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS | ||
| ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase()) | ||
| : []; | ||
|
|
||
| const uniqueGuests = guests.filter( | ||
| (guest) => | ||
| !booking.attendees.some((attendee) => guest === attendee.email) && | ||
| !blacklistedGuestEmails.includes(guest) | ||
| ); | ||
|
|
||
| if (uniqueGuests.length === 0) | ||
| throw new TRPCError({ code: "BAD_REQUEST", message: "emails_must_be_unique_valid" }); |
There was a problem hiding this comment.
Normalize and dedupe guest emails before filtering.
Without normalization and deduping, case-variant duplicates slip through and blacklisted emails can bypass checks.
🛡️ Suggested fix
- const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
- ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase())
- : [];
-
- const uniqueGuests = guests.filter(
- (guest) =>
- !booking.attendees.some((attendee) => guest === attendee.email) &&
- !blacklistedGuestEmails.includes(guest)
- );
+ const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
+ ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase())
+ : [];
+
+ const existingAttendeeEmails = new Set(
+ booking.attendees.map((attendee) => attendee.email.toLowerCase())
+ );
+
+ const normalizedGuests = guests.map((guest) => guest.trim().toLowerCase());
+ const uniqueGuests = Array.from(new Set(normalizedGuests)).filter(
+ (guest) => !existingAttendeeEmails.has(guest) && !blacklistedGuestEmails.includes(guest)
+ );🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts` around
lines 70 - 81, Normalize and dedupe incoming guest emails before computing
uniqueGuests: trim and lowercase each entry in guests (and derive a Set to
remove duplicates), compare against booking.attendees by normalizing
attendee.email to lowercase as well, and check against blacklistedGuestEmails
(which should also be normalized to lowercase/trimmed) so case-variant
duplicates or blacklisted variants are correctly rejected; update the logic
around uniqueGuests and the subsequent TRPCError throw to use these
normalized/deduped values.
Comment on lines
+167
to
+168
| try { | ||
| await sendAddGuestsEmails(evt, guests); |
There was a problem hiding this comment.
Send emails only to validated/added guests.
Using the original guests array can notify blacklisted, duplicate, or already-attending emails.
📧 Suggested fix
- await sendAddGuestsEmails(evt, guests);
+ await sendAddGuestsEmails(evt, uniqueGuests);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| await sendAddGuestsEmails(evt, guests); | |
| try { | |
| await sendAddGuestsEmails(evt, uniqueGuests); |
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts` around
lines 167 - 168, The code is sending emails using the original guests array
which may contain blacklisted, duplicate, or already-attending entries; change
the call to sendAddGuestsEmails to pass only the guests that were actually
validated/added (e.g., the result/variable returned by your add-guests logic
such as addedGuests or validatedGuests) by filtering out
blacklisted/duplicate/already-attending entries before invoking
sendAddGuestsEmails(evt, ...), or update sendAddGuestsEmails to accept and use
the validated/added list produced by the addGuests handler.
Comment on lines
+20
to
+25
| <div> | ||
| <label htmlFor="emails" className="text-default my-2 block text-sm font-medium"> | ||
| {label} | ||
| </label> | ||
| <ul> | ||
| {value.map((field, index) => ( |
There was a problem hiding this comment.
Associate the label with the email fields for accessibility.
htmlFor="emails" doesn’t reference any input id, so screen readers won’t link the label to the fields.
🛠️ Suggested fix (fieldset/legend)
- <div>
- <label htmlFor="emails" className="text-default my-2 block text-sm font-medium">
- {label}
- </label>
- <ul>
+ <div>
+ <fieldset>
+ <legend className="text-default my-2 block text-sm font-medium">{label}</legend>
+ <ul>
{value.map((field, index) => (
<li key={index}>
<EmailField
disabled={readOnly}
@@
}
/>
</li>
))}
- </ul>
+ </ul>
+ </fieldset>
{!readOnly && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div> | |
| <label htmlFor="emails" className="text-default my-2 block text-sm font-medium"> | |
| {label} | |
| </label> | |
| <ul> | |
| {value.map((field, index) => ( | |
| <div> | |
| <fieldset> | |
| <legend className="text-default my-2 block text-sm font-medium">{label}</legend> | |
| <ul> | |
| {value.map((field, index) => ( |
🤖 Prompt for AI Agents
In `@packages/ui/form/MultiEmail.tsx` around lines 20 - 25, The label with
htmlFor="emails" is not linked to any input; update the MultiEmail component to
use a fieldset/legend for accessibility: replace the standalone <label
htmlFor="emails"> with a <fieldset> that contains a <legend>{label}</legend>,
wrap the mapped email inputs (the value.map rendering) inside that fieldset, and
ensure each rendered input remains focusable and has any needed aria attributes
(or ids) so screen readers correctly associate the legend with the email fields.
Sign in
to join this conversation on GitHub.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit